Pass over a declared source root that is not there - #220

Summary

A source root that a build file names but the project does not contain ended every test in the run. Gradle and Maven both treat such a directory as legal and simply take nothing from it, so Ares now passes it over, says which one it passed over, and records that the source set is no longer known to be the whole project. A path that exists without being a directory, and one outside the project, are refused exactly as before.

Linked issues

No linked issues.

1. Problem

Discovery refused any declared source root that was not an existing directory. That is right for a path that exists and is not a directory, and ProjectSourcesFinderEdgeCaseTest pins that case down. It also fired for a root that is simply not there, which no build tool treats as an error.

ProjectSourcesFinder is at fault, in validateSourceRoot and its callers. Nothing catches the refusal, so JUnit records it against every policy-protected test.

Observed in ls1intum/SCOREReproducibilityPackage on JDK 17 and Gradle 9.7.1, upgrading 2.1.2 to 2.1.3: its build file declares assignment/src, a path that exists only once a student repository is checked out there. 220 of 294 tests in one run failed with the same IllegalStateException, all eight container images and every mode job red.

Nothing was recorded wrongly, since the run ends before any attack executes. What was lost is the run itself, and a diagnosis a reader had to dig a stack trace out of a test report to find.

2. Improvement from the user's perspective

An exercise whose build file names a directory that is optional, generated later, or checked out only in some runs now works instead of failing wholesale. That covers a test repository exercised on its own and any build declaring a generated source directory whose producer legitimately writes nothing.

Where a root is passed over, the log names it and the build file it came from, so the fix is one line in that file rather than a stack trace to interpret.

Enforcement does not become more permissive. The roots that are there still scope what Ares supervises, and because the source set is marked as not known to be whole, the supervised package is taken from the compiled output instead of being counted across what may be part of a project.

3. Improvement from the maintainer's perspective

One rule now says what it means. Files.isDirectory was false both for a path that is absent and for one occupied by a file, and only the second is a configuration error. Those are separate conditions in separate methods, so each can be read and tested on its own.

Two latent faults on the Maven side go with it. A declared <sourceDirectory> that was passed over would have emptied the list and silently reactivated src/main/java, which is the substitution the old refusal existed to prevent; the reader now tracks that something was declared, apart from what it resolved to. And the broad catch around the whole of Maven reading rewrote every refusal as Cannot parse Maven source roots, including the SecurityException for a root outside the project; it now covers reading the file alone.

Maven can also report incomplete roots now, where it previously hard-coded completeness.

4. Testing manual

Build tool independent: the change is in discovery, which runs before any mode is chosen. Gradle and Maven are both exercised below.

Prerequisites

1. A checkout of this branch and a JDK 21. Maven comes from the wrapper.
2. Nothing else. These steps do not install Ares into your local Maven repository, deliberately: this branch carries version 2.1.3, so installing it would replace the released 2.1.3 for every other project on the machine.

Steps

1. Run the case this change is about: mvn test -Dtest=ProjectSourcesFinderAbsentRootTest.
2. Read the build output for the lines the finder logs while those tests run.
3. Run the cases that must not have moved: mvn test -Dtest=ProjectSourcesFinderEdgeCaseTest,ProjectSourcesFinderTest.
4. Run the consumer of the completeness flag: mvn test -Dtest=JavaProjectScannerPackageFallbackTest.

Expected result

Step 1: 14 tests pass. Read their names in the output rather than the count: they state the contract, including that a replacement naming one present and one absent root keeps the present one, that a replacement naming only an absent root leaves the set empty rather than falling back to src/main/java, that a root outside the project is still refused, and that a link pointing at nothing is passed over while one pointing at a directory is followed and checked on what it resolves to.

Step 2: for each passed-over root, a line naming that root and the build file that declared it, then one line per source set saying its roots are no longer known to be the whole of it. A run that passes a root over silently would be wrong, and so would one that names no descriptor.

Step 3: 52 tests pass. Two matter most, and both assert on a refusal that must survive: rejectsSourceRootThatIsNotADirectory writes a file and declares it as a root, and rejectsGradleRootEscapingTheProject declares one outside the project.

Step 4: 10 tests pass, including ignoresAPresentSourceRootReportedAsIncomplete. It puts a real source root holding package somewhere.other.entirely next to compiled output holding de.tum.cit.aet, marks the roots incomplete, and requires the compiled output to win. Flipping that flag to true makes it fail with somewhere.other.entirely, which is what shows the test is about the flag rather than about the fixture.

Negative case (what must still be rejected)

Three refusals must survive, and steps 1 and 3 each check some of them: a declared root that exists and is not a directory, a declared root resolving outside the project, and a Maven root outside the project, which must arrive as SecurityException rather than as a parsing failure. ProjectSourcesFinderAbsentRootTest covers the Maven half, ProjectSourcesFinderEdgeCaseTest the Gradle half.

Ares has become more permissive in exactly one respect, deliberately, and it is worth a reviewer's attention: a masking defect, where an example inside a comment is read as a declaration, used to end the run loudly. It now empties the roots and marks them incomplete. The two are indistinguishable once the reader has produced the same declaration, so what defends against a decoy is correct masking and the tests for it, ignoresSourceDirectoriesInsideCommentsAndStrings and ignoresSourceDirectoriesInsideASlashyString, both of which assert on the roots and both of which still pass.

Modes exercised

No mode-specific behaviour changed.

5. Test case coverage regarding this PR

Figures from a local mvn test -Punit-core-tests,coverage on this branch, since the aggregated CI report is not available before the run. BuildToolConfiguration and JavaProjectScanner carry Javadoc corrections only and have no row.

Every new test asserts on the discovered roots and the completeness flag, not on a run finishing. The scanner test was confirmed to fail when the flag it is about is flipped.

Breaking changes and migration

No breaking changes or migration. The public API under de.tum.cit.ase.ares.api is unchanged, and so are the policy file format, the generated security test code and the minimum JDK, Maven and Gradle versions.

The behaviour change goes in the permissive direction only: an exercise that failed on 2.1.3 because a declared source root was absent now runs. An exercise that worked before works unchanged, since a root that was there was never passed over. Nothing has to be done to upgrade.

Checklist

• The title of this pull request describes the change, not the implementation.
• I followed the guidelines for inclusive, diversity-sensitive and appreciative language.
• I have self-reviewed the diff of this pull request.
• Tests were added or updated for the behaviour changed here.
• Javadoc follows the AGENTS.md conventions.
• Documentation (docs/, README.adoc, Javadoc) was updated where the change is user-facing.
• CI is green, or every remaining failure is explained above.
• No secrets, tokens or absolute local paths are contained in the diff.

Review progress

• Code review
• Manual test